Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: remove unused variables from HTTPS tests #4426

Closed
wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Dec 26, 2015

No description provided.

@Trott Trott added https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests. labels Dec 26, 2015
@Trott
Copy link
Member Author

Trott commented Dec 26, 2015

@jbergstroem
Copy link
Member

(sorry for the slight delay in testing; redeploying machines so ccache is cold)

@jbergstroem
Copy link
Member

Is the failed test on windows (tick processor) a known issue/documented somewhere? Obviously unrelated here.

@jbergstroem
Copy link
Member

LGTM

@Trott
Copy link
Member Author

Trott commented Dec 26, 2015

Unfortunately, that seemingly unrelated test failure looks like a new one. Running a stress test to see if it's easily reproducible: https://ci.nodejs.org/job/node-stress-single-test/208/nodes=win-vs2013/console

And just for good measure, re-running CI to confirm that it's not some mysterious "oh yeah, if you change a test over here, it affects a test over there" kind of thing that will definitely make me question everything I think I know. Tell me, Node.js CI, is my entire existence a lie?! https://ci.nodejs.org/job/node-test-commit/1534/

@Trott
Copy link
Member Author

Trott commented Dec 26, 2015

Added an issue for the failed test: #4427

@Trott
Copy link
Member Author

Trott commented Dec 26, 2015

One more var removed, re-running CI: https://ci.nodejs.org/job/node-test-commit/1537/

Trott added a commit to Trott/io.js that referenced this pull request Dec 29, 2015
PR-URL: nodejs#4426
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Trott
Copy link
Member Author

Trott commented Dec 29, 2015

Landed in fd395ba

@Trott Trott closed this Dec 29, 2015
Trott added a commit to Trott/io.js that referenced this pull request Dec 29, 2015
PR-URL: nodejs#4426
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
PR-URL: nodejs#4426
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Jan 14, 2016
PR-URL: nodejs#4426
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
PR-URL: #4426
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@MylesBorins MylesBorins mentioned this pull request Jan 19, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4426
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@Trott Trott deleted the rm-unused-https branch January 13, 2022 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
https Issues or PRs related to the https subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants